Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new AVP::AnimationOptimization property #6269

Merged
merged 12 commits into from
Jan 25, 2022

Conversation

aborziak-ms
Copy link
Contributor

@aborziak-ms aborziak-ms commented Nov 10, 2021

Description

In this change we are adding AnimationOptimization property to the AnimatedVisualPlayer. This property has two possible values: Resources or Latency (default).
When animation is completed and optimization mode is set to Resources we will destroy all the windows composition animations from the lottie side. When we call PlayAsync or SetProgress we will re-instantiate all the animations again.
If optimization mode is set to Latency we will immediately instantiate all the animations and will keep them even if AVP is not playing anything.

Also in order to expose this functionality from LottieGen side we need to add IAnimatedVisual and IAnimatedVisualSource3 interfaces that are used only by AVP.

Motivation and Context

There are some performance issues with the current implementation of LottieGen and AnimatedVisual Player:

  1. Complex Lottie animations can instantiate a lot of composition animations which may impact performance and overall battery life.
  2. Lottie files that have a lot of complex animations even when they are paused, have some cost to performance.
    The Idle (paused) lottie animations use the same amount of CPU resources as when they are playing the actual animation.

#6268

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Nov 10, 2021
auto clampedProgress = std::clamp(static_cast<float>(progress), 0.0F, 1.0F);

// WARNING: Reentrance via IsPlaying DP may occur from this point down to the end of the method
// iff m_nowPlaying.

// Setting the Progress value will stop the current play.
m_progressPropertySet.InsertScalar(L"Progress", static_cast<float>(clampedProgress));
m_lastPlayProgress = progress;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_lastPlayProgress = progress

m_lastPlayProgress will be clamped later when it is used? (i.e. in stop?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, probably it is better to set it to clampedProgress here, thx

Copy link

@raymond-wh-leung raymond-wh-leung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@ranjeshj ranjeshj added team-CompInput Issue for IXP (Composition, Input) team area-AnimatedVisualPlayer labels Nov 10, 2021
@ranjeshj
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -62,6 +88,9 @@ unsealed runtimeclass AnimatedVisualPlayer
[MUX_DEFAULT_VALUE("true")]
[MUX_PROPERTY_CHANGED_CALLBACK(TRUE)]
Boolean AutoPlay;
[MUX_DEFAULT_VALUE("winrt::AnimationsCacheModeEnum::Always")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would need to version these APIs since you are adding to an existing type. See MUX_PUBLIC_V2 to see how it is done in other types. @kmahone as FYI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done

aborziak-ms and others added 2 commits January 18, 2022 14:12
@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

if (m_owner)
{
// If optimization is set to Resources - destroy animations immediately after player stops.
if (m_owner->AnimatedVisualPlayerProperties::AnimationOptimization() == winrt::PlayerAnimationOptimization::Resources)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AnimatedVisualPlayerProperties::AnimationOptimization

Being explicit in this way is non-intuitive. The other cases where this ambiguity has come up we've put this in the class in the header file:

using AnimatedVisualPlayerProperties::AnimationOptimization;

@aborziak-ms aborziak-ms changed the title Added new AVP::AnimationCacheMode API Added new AVP::AnimationOptimization property Jan 20, 2022
@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

}

// Call RequestCommit to make sure that previous compositor calls complete before destroying animations.
m_rootVisual.Compositor().RequestCommitAsync().Completed(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RequestCommitAsync() is only available on Rs4+ we need to gracefully handle the case where this API isn't available on RS2 and RS3.

{
return;
}

// Call RequestCommit to make sure that previous compositor calls complete before destroying animations.
// RequestCommitAsync is available only for RS4+ but m_animatedVisual is not null guarantees that we are at least RS5+
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this the case? Don't the visuals get created at startup? and then I could call destroy animations on this and hit this code path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destroy and Create are private inside AVP, we are exposing only AnimationOptimization property

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters merged commit 8d59449 into microsoft:main Jan 25, 2022
@krschau krschau removed the needs-triage Issue needs to be triaged by the area owners label Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-AnimatedVisualPlayer team-CompInput Issue for IXP (Composition, Input) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants